Skip to content

fix: use plain strings for Keycloak client attributes in update payload#324

Merged
pdettori merged 1 commit into
rossoctl:mainfrom
akram:fix/keycloak-attr-string-values
May 7, 2026
Merged

fix: use plain strings for Keycloak client attributes in update payload#324
pdettori merged 1 commit into
rossoctl:mainfrom
akram:fix/keycloak-attr-string-values

Conversation

@akram

@akram akram commented May 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • mergeDesiredClientIntoMap was wrapping attribute values in []interface{} arrays (e.g. ["false"] instead of "false")
  • Keycloak Admin API rejects this with 400: Cannot parse the JSON on PUT
  • This caused operator-managed client registration to silently fail for all workloads when the Keycloak client already existed and needed an update
  • Credential secrets were never created, leaving pods stuck in Init:0/1 waiting for volume mounts

Root Cause

In internal/keycloak/admin.go, mergeDesiredClientIntoMap line 414:

// Before (broken):
attrs[k] = []interface{}{v}

// After (fixed):
attrs[k] = v

How it was found

Discovered while testing PR #321 on an OpenShift cluster. The operator authenticated to Keycloak successfully but every client update returned 400 "Cannot parse the JSON". Reproduced by manually PUTting the same payload to the Keycloak Admin API — array-wrapped attributes fail, plain string attributes succeed.

Test plan

  • go test ./internal/keycloak/... — all 11 tests pass (updated TestAdmin_RegisterOrFetchClient_updatesDrift)
  • Verified on live OpenShift cluster: client registration succeeds, credential secrets created, pods start

@akram
akram requested a review from a team as a code owner May 4, 2026 13:59
@akram
akram force-pushed the fix/keycloak-attr-string-values branch from 407b699 to d320915 Compare May 4, 2026 14:03
mergeDesiredClientIntoMap was wrapping attribute values in []interface{}
arrays (e.g. ["false"] instead of "false"), causing Keycloak Admin API
to reject the PUT request with 400 "Cannot parse the JSON".

Keycloak expects attribute values as plain strings, not arrays. This
caused operator-managed client registration to fail for all workloads
when the client already existed in Keycloak and needed an update.

Signed-off-by: Akram Ben Aissi <akram.benaissi@gmail.com>

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Akram <akram.benaissi@gmail.com>
@akram
akram force-pushed the fix/keycloak-attr-string-values branch from d320915 to 5596d7c Compare May 4, 2026 14:05

@cwiklik cwiklik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean one-line fix for a real bug — Keycloak rejects array-wrapped attributes with 400. Root cause well-documented, test updated, all CI green.

Areas reviewed: Go
Commits: 1, signed-off
CI: all passing

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@pdettori
pdettori merged commit 02569a9 into rossoctl:main May 7, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants